Conversation
📝 WalkthroughWalkthroughProject-local schemas at /openspec/schemas are added to the resolver and now take precedence over user and package schemas. The resolver exposes Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Resolver
participant Project as ProjectDir (cwd/openspec/schemas)
participant User as UserDir (XDG_DATA_HOME/openspec/schemas)
participant Package as PackageSchemas
Client->>Project: check for <name>/schema.yaml
alt found
Project-->>Client: return project schema (source: project)
else not found
Client->>User: check for <name>/schema.yaml
alt found
User-->>Client: return user schema (source: user)
else not found
Client->>Package: check for <name>/schema.yaml
Package-->>Client: return package schema (source: package) or not found
end
end
sequenceDiagram
participant Lister as listSchemasWithInfo
participant Project as ProjectDir (cwd/openspec/schemas)
participant User as UserDir (XDG_DATA_HOME/openspec/schemas)
participant Package as PackageSchemas
Lister->>Project: enumerate & parse schemas -> add as 'project'
Lister->>User: enumerate & parse -> add as 'user' if name not seen
Lister->>Package: enumerate & parse -> add as 'package' if name not seen
Lister-->>Client: return deduplicated, ordered list with sources
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
🧬 Code graph analysis (2)test/core/artifact-graph/resolver.test.ts (2)
src/core/artifact-graph/resolver.ts (1)
🔇 Additional comments (10)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/artifact-graph/resolver.ts (1)
82-92: JSDoc is inconsistent with implementation.The documentation still describes only 2 resolution levels (user → package), but since
getSchemaDir()now includes project-local schemas, the actual resolution order is project → user → package.📝 Proposed fix
/** * Resolves a schema name to a SchemaYaml object. * * Resolution order: - * 1. User override: ${XDG_DATA_HOME}/openspec/schemas/<name>/schema.yaml - * 2. Package built-in: <package>/schemas/<name>/schema.yaml + * 1. Project-local: <cwd>/openspec/schemas/<name>/schema.yaml + * 2. User override: ${XDG_DATA_HOME}/openspec/schemas/<name>/schema.yaml + * 3. Package built-in: <package>/schemas/<name>/schema.yaml * * `@param` name - Schema name (e.g., "spec-driven") * `@returns` The resolved schema object * `@throws` Error if schema is not found in any location */
🧹 Nitpick comments (2)
src/core/artifact-graph/resolver.ts (1)
222-224: Consider adding debug logging for skipped schemas.Silent failures here could make troubleshooting difficult when a schema unexpectedly doesn't appear in listings. A debug-level log would aid diagnostics without affecting user experience.
💡 Example
} catch { - // Skip invalid schemas + // Skip invalid schemas - log for debugging + // console.debug(`Skipping invalid project schema: ${entry.name}`); }test/core/artifact-graph/resolver.test.ts (1)
140-194: Consider adding a test forresolveSchemawith project-local schemas.The existing
resolveSchematests cover user override and package fallback, but there's no explicit test for project-local schema resolution. WhilegetSchemaDirprecedence is tested separately, an integration test forresolveSchemawith project-local would improve confidence.💡 Example test
it('should prefer project-local over user override', () => { process.env.XDG_DATA_HOME = tempDir; const userSchemaDir = path.join(tempDir, 'openspec', 'schemas', 'spec-driven'); fs.mkdirSync(userSchemaDir, { recursive: true }); fs.writeFileSync(path.join(userSchemaDir, 'schema.yaml'), ` name: user-override version: 50 artifacts: - id: user-artifact generates: user.md description: User artifact template: user.md `); const projectSchemaDir = path.join(tempDir, 'project', 'openspec', 'schemas', 'spec-driven'); fs.mkdirSync(projectSchemaDir, { recursive: true }); fs.writeFileSync(path.join(projectSchemaDir, 'schema.yaml'), ` name: project-local version: 100 artifacts: - id: project-artifact generates: project.md description: Project artifact template: project.md `); const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(path.join(tempDir, 'project')); try { const schema = resolveSchema('spec-driven'); expect(schema.name).toBe('project-local'); expect(schema.version).toBe(100); } finally { cwdSpy.mockRestore(); } });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/artifact-graph/resolver.tstest/core/artifact-graph/resolver.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
test/core/artifact-graph/resolver.test.ts (1)
src/core/artifact-graph/resolver.ts (3)
getProjectSchemasDir(35-37)listSchemas(143-186)listSchemasWithInfo(202-278)
src/core/artifact-graph/resolver.ts (2)
src/core/artifact-graph/index.ts (3)
getSchemaDir(25-25)getUserSchemasDir(27-27)parseSchema(12-12)src/core/artifact-graph/schema.ts (1)
parseSchema(23-45)
🔇 Additional comments (10)
src/core/artifact-graph/resolver.ts (5)
32-37: LGTM!The
getProjectSchemasDir()function correctly usesprocess.cwd()to resolve the project-local schemas directory path relative to the current working directory.
57-80: LGTM!The resolution order correctly implements project-local → user → package precedence, matching the updated JSDoc.
143-186: LGTM!The
Setcorrectly deduplicates schema names across all three sources. The processing order doesn't affect the output since this function only collects unique names without tracking source.
191-196: LGTM!The
SchemaInfointerface correctly includes'project'in the source union type.
206-228: LGTM!The project-local schemas are correctly processed first with highest precedence. The
seenNamestracking ensures proper deduplication.test/core/artifact-graph/resolver.test.ts (5)
1-14: LGTM!Imports correctly include the new
listSchemasWithInfoandgetProjectSchemasDirexports.
47-62: LGTM!Good test coverage for
getProjectSchemasDir()including mock-based verification that the function respectsprocess.cwd(). Thetry/finallypattern ensures proper mock cleanup.
89-139: LGTM!Excellent coverage for project-local precedence in
getSchemaDir:
- Verifies project-local wins over user override
- Verifies fallback to user when project-local doesn't have the schema
The test setup is clean and mock cleanup is properly handled.
395-431: LGTM!Good test coverage for
listSchemas()with project-local schemas:
- Verifies project-local schemas are included in listings
- Verifies deduplication works correctly across all sources
434-576: LGTM!Comprehensive test suite for
listSchemasWithInfo()covering:
- Source field verification for all three sources (package, user, project)
- Precedence handling (project > user > package)
- Sorting behavior
- Error tolerance for invalid schemas
The tests are well-structured with proper mock cleanup.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/artifact-graph/resolver.ts (1)
82-92: Stale JSDoc: resolution order is outdated.The JSDoc for
resolveSchema()still documents the old resolution order (user → package) but should reflect the new order (project → user → package) since it callsgetSchemaDir()internally.📝 Suggested fix
/** * Resolves a schema name to a SchemaYaml object. * * Resolution order: - * 1. User override: ${XDG_DATA_HOME}/openspec/schemas/<name>/schema.yaml - * 2. Package built-in: <package>/schemas/<name>/schema.yaml + * 1. Project-local: <cwd>/openspec/schemas/<name>/schema.yaml + * 2. User override: ${XDG_DATA_HOME}/openspec/schemas/<name>/schema.yaml + * 3. Package built-in: <package>/schemas/<name>/schema.yaml * * `@param` name - Schema name (e.g., "spec-driven") * `@returns` The resolved schema object * `@throws` Error if schema is not found in any location */
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/experimental-workflow.mdsrc/core/artifact-graph/resolver.tstest/core/artifact-graph/resolver.test.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
Applied to files:
docs/experimental-workflow.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
Applied to files:
docs/experimental-workflow.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/{proposal,design,tasks}.md : Use direct file references in format `file.ts:42` and reference specs as `specs/auth/spec.md` for clarity
Applied to files:
src/core/artifact-graph/resolver.ts
🧬 Code graph analysis (2)
test/core/artifact-graph/resolver.test.ts (2)
src/core/artifact-graph/resolver.ts (4)
getProjectSchemasDir(35-37)getSchemaDir(57-80)listSchemas(143-186)listSchemasWithInfo(202-278)src/core/artifact-graph/index.ts (3)
getSchemaDir(25-25)listSchemas(23-23)listSchemasWithInfo(24-24)
src/core/artifact-graph/resolver.ts (1)
src/core/artifact-graph/schema.ts (1)
parseSchema(23-45)
🔇 Additional comments (10)
docs/experimental-workflow.md (1)
476-476: LGTM!The documentation correctly reflects the new project-level schema path and aligns with the implementation in
getProjectSchemasDir(). The parenthetical clarification helps users understand both options.src/core/artifact-graph/resolver.ts (4)
32-37: LGTM!The implementation correctly uses
process.cwd()to resolve the project-local schemas directory, making it context-aware to the current working directory.
46-79: LGTM!The precedence implementation (project → user → package) is correct, with early returns for efficiency. The JSDoc accurately documents the resolution order.
139-186: LGTM!The
Set-based deduplication correctly collects all unique schema names regardless of insertion order. The function properly scans all three schema sources.
188-277: LGTM!The implementation correctly:
- Extends
SchemaInfo.sourceto include'project'- Processes schemas in correct precedence order (project → user → package)
- Uses
seenNamesfor deduplication, ensuring only the highest-precedence schema is included- Handles invalid schemas gracefully with try-catch
test/core/artifact-graph/resolver.test.ts (5)
1-14: LGTM!Imports are correctly updated to include
vifrom vitest for mocking, and the new public APIs (listSchemasWithInfo,getProjectSchemasDir) are imported for testing.
47-62: LGTM!Good test coverage for
getProjectSchemasDir():
- Verifies correct path construction relative to
process.cwd()- Tests dynamic behavior when cwd changes via mock
- Properly restores mocks to prevent test pollution
89-138: LGTM!Excellent test coverage for the new precedence behavior:
- Verifies project-local takes precedence over user override
- Verifies fallback to user override when project-local schema doesn't exist
- Proper mock management with try/finally blocks
395-431: LGTM!Tests properly verify:
- Project-local schemas are included in the listing
- Deduplication works correctly when same schema name exists across project, user, and package sources
434-576: LGTM!Comprehensive test suite for
listSchemasWithInfo():
- Validates source field values (
'package','user','project')- Confirms correct precedence (project > user > package)
- Verifies metadata extraction (description, artifacts)
- Tests sorted output
- Ensures invalid schemas are silently skipped without throwing
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@CS-Tao Haha I'm working on the same thing here: https://github.com/Fission-AI/OpenSpec/pull/499/changes#diff-3f104fb5dfabd94e8105edd5ebab1d621c5ce037dd2e77a6ab7b44bed6c4cb3a I'll take a look at your implementation of it to see if it matches the spec I had in mind |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
|
Thanks for putting this together! 🙏 This is a great feature idea, and we actually ended up implementing the same functionality in PR #522 (merged in The current implementation supports everything you proposed here:
Closing this as superseded, but really appreciate you identifying this need and drafting a solution! |
|
@TabishB When will the new version be released? |
|
@CS-Tao Tonight (in 2 hours), but treat it as beta/experimental - if something breaks let me know |
|
@CS-Tao Also would be keen to chat about how you're using OpenSpec and what kind of features you wish it had. Would love to get on a call if possible! |
|
@CS-Tao new version released, available in v0.22.0 more info in docs at docs/experimental-workflow.md |
2 participants
Summary
Adds support for project-level custom schemas stored in
<project>/openspec/schemas/. This allows teams to define workflow schemas that are specific to their project without needing to install them globally.Changes
Modified files:
src/core/artifact-graph/resolver.ts- AddgetProjectSchemasDir()function; update schema resolution order to prioritize project-local schemas (project → user → package); extendlistSchemas()andlistSchemasWithInfo()to include project-local schemas; add'project'as a new source type inSchemaInfotest/core/artifact-graph/resolver.test.ts- Add comprehensive tests for project-local schema resolution, precedence ordering, andlistSchemasWithInfo()functionalitydocs/experimental-workflow.md- Document the new project-level schemas locationResolution Order
Schema resolution now follows this precedence (highest to lowest):
<cwd>/openspec/schemas/<name>/schema.yaml${XDG_DATA_HOME}/openspec/schemas/<name>/schema.yaml<package>/schemas/<name>/schema.yaml🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.